reduce redundancy in bubbles #1062
Conversation
|
CodeAnt AI is reviewing your PR. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughMove vapor diffusion coefficient from Lagrangian bubble parameter Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Config as Config / Examples
participant Pre as Pre-process
participant MPI as MPI proxy
participant Sim as Simulation
participant Phys as m_helper / m_bubbles_EL
rect rgb(240,248,255)
Config->>Pre: supply `fluid_pp(2)%D_v` or use default
Pre->>MPI: include `fluid_pp(i)%D_v` in broadcast set
MPI->>Sim: broadcast `fluid_pp(i)%D_v` to all ranks
end
rect rgb(245,255,240)
Sim->>Phys: request diffusivity for computations
Phys-->>Sim: use `fluid_pp(... )%D_v` in Pe, betaC, D_m calculations
end
note right of Phys `#f7f7c9`: `lag_params%diffcoefvap` removed — replaced by `fluid_pp% D_v`
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
qodo-code-review
bot
changed the title
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
codeant-ai
bot
added
the
size:S
codeant-ai
bot
changed the title
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/simulation/m_bubbles_EL.fpp (1)
389-391: Migration from lag_params to fluid_pp is correct, but consider using id_bubbles for consistency.The replacement of
lag_params%diffcoefvapwithfluid_pp(num_fluids)%D_vproperly implements the migration to per-fluid vapor diffusivity parameters. However, there is an inconsistency in identifier usage:
- Line 181 in
s_start_lagrange_inputsusesid_bubbles(whereid_bubbles = num_fluids)- Lines 389 and 391 in
s_add_bubblesusenum_fluidsdirectlyFor improved clarity and maintainability, consider using
id_bubblesconsistently throughout both subroutines, as suggested in the previous review comment. This would make the code's intent more explicit: accessing bubble gas properties specifically, rather than "the last fluid."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
examples/1D_qbmm/case.py(2 hunks)examples/3D_lagrange_bubblescreen/case.py(2 hunks)examples/3D_lagrange_shbubcollapse/case.py(2 hunks)src/common/m_derived_types.fpp(1 hunks)src/common/m_helper.fpp(1 hunks)src/post_process/m_global_parameters.fpp(1 hunks)src/post_process/m_mpi_proxy.fpp(1 hunks)src/pre_process/m_global_parameters.fpp(1 hunks)src/pre_process/m_mpi_proxy.fpp(1 hunks)src/simulation/m_bubbles_EL.fpp(2 hunks)src/simulation/m_global_parameters.fpp(1 hunks)src/simulation/m_mpi_proxy.fpp(2 hunks)toolchain/mfc/run/case_dicts.py(4 hunks)toolchain/mfc/test/case.py(0 hunks)toolchain/mfc/test/cases.py(2 hunks)
💤 Files with no reviewable changes (1)
- toolchain/mfc/test/case.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{fpp,f90}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{fpp,f90}: Use 2-space indentation; continuation lines align beneath &
Use lower-case keywords and intrinsics (do, end subroutine, etc.)
Name modules with m_ pattern (e.g., m_transport)
Name public subroutines with s_ pattern (e.g., s_compute_flux)
Name public functions with f_ pattern
Keep subroutine size ≤ 500 lines, helper subroutines ≤ 150 lines, functions ≤ 100 lines, files ≤ 1000 lines
Limit routine arguments to ≤ 6; use derived-type params struct if more are needed
Forbid goto statements (except in legacy code), COMMON blocks, and save globals
Every argument must have explicit intent; use dimension/allocatable/pointer as appropriate
Call s_mpi_abort() for errors, never use stop or error stop
**/*.{fpp,f90}: Indent 2 spaces; continuation lines align under&
Use lower-case keywords and intrinsics (do,end subroutine, etc.)
Name modules withm_<feature>prefix (e.g.,m_transport)
Name public subroutines ass_<verb>_<noun>(e.g.,s_compute_flux) and functions asf_<verb>_<noun>
Keep private helpers in the module; avoid nested procedures
Enforce size limits: subroutine ≤ 500 lines, helper ≤ 150, function ≤ 100, module/file ≤ 1000
Limit subroutines to ≤ 6 arguments; otherwise pass a derived-type 'params' struct
Avoidgotostatements (except unavoidable legacy); avoid global state (COMMON,save)
Every variable must haveintent(in|out|inout)specification and appropriatedimension/allocatable/pointer
Uses_mpi_abort(<msg>)for error termination instead ofstop
Use!>style documentation for header comments; follow Doxygen Fortran format with!! @paramand!! @returntags
Useimplicit nonestatement in all modules
Useprivatedeclaration followed by explicitpublicexports in modules
Use derived types with pointers for encapsulation (e.g.,pointer, dimension(:,:,:) => null())
Usepureandelementalattributes for side-effect-free functions; combine them for array ...
Files:
src/common/m_derived_types.fppsrc/post_process/m_mpi_proxy.fppsrc/common/m_helper.fppsrc/simulation/m_global_parameters.fppsrc/pre_process/m_mpi_proxy.fppsrc/pre_process/m_global_parameters.fppsrc/simulation/m_mpi_proxy.fppsrc/post_process/m_global_parameters.fppsrc/simulation/m_bubbles_EL.fpp
src/**/*.fpp
📄 CodeRabbit inference engine (.cursor/rules/mfc-agent-rules.mdc)
src/**/*.fpp: Use.fppfile extension for Fypp preprocessed files; CMake transpiles them to.f90
Start module files with Fypp include for macros:#:include 'macros.fpp'
Use the fyppASSERTmacro for validating conditions:@:ASSERT(predicate, message)
Use fypp macro@:ALLOCATE(var1, var2)for device-aware allocation instead of standard Fortranallocate
Use fypp macro@:DEALLOCATE(var1, var2)for device-aware deallocation instead of standard Fortrandeallocate
Files:
src/common/m_derived_types.fppsrc/post_process/m_mpi_proxy.fppsrc/common/m_helper.fppsrc/simulation/m_global_parameters.fppsrc/pre_process/m_mpi_proxy.fppsrc/pre_process/m_global_parameters.fppsrc/simulation/m_mpi_proxy.fppsrc/post_process/m_global_parameters.fppsrc/simulation/m_bubbles_EL.fpp
src/simulation/**/*.{fpp,f90}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/simulation/**/*.{fpp,f90}: Wrap tight GPU loops with !$acc parallel loop gang vector default(present) reduction(...); add collapse(n) when safe; declare loop-local variables with private(...)
Allocate large GPU arrays with managed memory or move them into persistent !$acc enter data regions at start-up
Avoid stop/error stop inside GPU device code
Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers
src/simulation/**/*.{fpp,f90}: Mark GPU-callable helpers with$:GPU_ROUTINE(function_name='...', parallelism='[seq]')immediately after declaration
Do not use OpenACC or OpenMP directives directly; use Fypp macros fromsrc/common/include/parallel_macros.fppinstead
Wrap tight loops with$:GPU_PARALLEL_FOR(private='[...]', copy='[...]')macro; addcollapse=nfor safe nested loop merging
Declare loop-local variables withprivate='[...]'in GPU parallel loop macros
Allocate large arrays withmanagedor move them into a persistent$:GPU_ENTER_DATA(...)region at start-up
Do not placestoporerror stopinside device code
Files:
src/simulation/m_global_parameters.fppsrc/simulation/m_mpi_proxy.fppsrc/simulation/m_bubbles_EL.fpp
🧠 Learnings (7)
📚 Learning: 2025-11-24T21:50:16.684Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.684Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Wrap tight GPU loops with !$acc parallel loop gang vector default(present) reduction(...); add collapse(n) when safe; declare loop-local variables with private(...)
Applied to files:
src/post_process/m_mpi_proxy.fppsrc/pre_process/m_mpi_proxy.fppsrc/simulation/m_mpi_proxy.fpp
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Wrap tight loops with `$:GPU_PARALLEL_FOR(private='[...]', copy='[...]')` macro; add `collapse=n` for safe nested loop merging
Applied to files:
src/post_process/m_mpi_proxy.fppsrc/simulation/m_mpi_proxy.fpp
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to **/*.{fpp,f90} : Keep private helpers in the module; avoid nested procedures
Applied to files:
src/common/m_helper.fpp
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Declare loop-local variables with `private='[...]'` in GPU parallel loop macros
Applied to files:
src/pre_process/m_mpi_proxy.fppsrc/simulation/m_mpi_proxy.fpp
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Do not use OpenACC or OpenMP directives directly; use Fypp macros from `src/common/include/parallel_macros.fpp` instead
Applied to files:
src/simulation/m_mpi_proxy.fpp
📚 Learning: 2025-11-24T21:50:16.684Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.684Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers
Applied to files:
src/simulation/m_mpi_proxy.fpp
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to **/*.{fpp,f90} : Use `wp` (working precision) parameter from `m_precision_select` instead of hardcoded precision like `real*8`
Applied to files:
src/post_process/m_global_parameters.fpp
🪛 GitHub Actions: Lint Toolchain
examples/3D_lagrange_bubblescreen/case.py
[error] 1-1: pylint: invalid non-printable character U+000D (syntax-error) detected at line 1.
examples/3D_lagrange_shbubcollapse/case.py
[error] 1-1: pylint: invalid non-printable character U+000D (syntax-error) detected at line 1.
🪛 Ruff (0.14.5)
examples/3D_lagrange_bubblescreen/case.py
1-1: Shebang is present but file is not executable
(EXE001)
examples/3D_lagrange_shbubcollapse/case.py
1-1: Shebang is present but file is not executable
(EXE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Github (ubuntu, no-mpi, single, no-debug, false)
- GitHub Check: Github (ubuntu, mpi, no-debug, true)
- GitHub Check: Github (ubuntu, mpi, no-debug, false)
- GitHub Check: Github (ubuntu, mpi, debug, true)
- GitHub Check: Github (ubuntu, mpi, debug, false)
- GitHub Check: Code Cleanliness Check
- GitHub Check: Coverage Test on CodeCov
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Build & Publish
🔇 Additional comments (20)
examples/3D_lagrange_shbubcollapse/case.py (1)
181-181: D_v wiring for bubble gas fluid looks consistent with the new designUsing
diffVaporto populate"fluid_pp(2)%D_v"aligns with the refactor to treat vapor diffusivity as a per‑fluid property and to associate it with the bubble gas phase (second fluid). This matches the PR objective and keeps the example in sync with the Fortran-side changes.src/common/m_helper.fpp (2)
154-154: Good refactor: eliminates magic number.Replacing the hardcoded diffusion coefficient
0.242e-4_wpwithfluid_pp(2)%D_vimproves maintainability and aligns with the PR's goal to centralize vapor diffusivity.
161-162: The repository is currently inaccessible in the sandbox environment due to cloning failures. I'm unable to verify:
- Whether
R_uniis defined inm_constants- Whether
m_constantsis imported inm_helper.fpp- The actual context and scope of the code at lines 161-162
Unable to verify
R_unidefinition and accessibility due to repository access limitations.The review comment's verification request cannot be completed without access to the repository files. Manual verification of the following is required:
- Confirm
R_uniis declared inm_constantsmodule- Confirm
use m_constantsstatement exists inm_helper.fpp- Validate that
R_uniis properly accessible at the code locationsrc/common/m_derived_types.fpp (1)
360-361: LGTM!The addition of
D_vto thephysical_parameterstype is well-placed and consistent with existing fluid property fields.src/pre_process/m_global_parameters.fpp (1)
587-587: LGTM!The initialization of
D_vwithdflt_realis correct and consistent with other fluid parameter defaults in the loop.src/post_process/m_mpi_proxy.fpp (1)
136-136: LGTM!The MPI broadcast of
fluid_pp(i)%D_vcorrectly propagates the vapor diffusivity parameter to all processes, maintaining consistency with other fluid property broadcasts.src/pre_process/m_mpi_proxy.fpp (2)
147-149: LGTM!The addition of
D_vto the fluid physical parameters MPI broadcast ensures proper distribution across processes.
155-157: LGTM!The inclusion of
D_vin the simplex noise and fluid physical parameters broadcast maintains consistency with the first broadcast loop.examples/3D_lagrange_bubblescreen/case.py (1)
179-179: LGTM!The addition of
fluid_pp(2)%D_vcorrectly maps the vapor diffusivity to the gas fluid parameters, replacing the removeddiffcoefvapfrom lag_params.src/simulation/m_mpi_proxy.fpp (2)
143-146: LGTM!The removal of
diffcoefvapfrom the lag_params MPI broadcast aligns with the migration tofluid_pp(i)%D_v.
192-194: LGTM!The addition of
D_vto the fluid parameters MPI broadcast ensures proper synchronization of the vapor diffusivity across all processes.toolchain/mfc/test/cases.py (2)
521-523: LGTM! D_v parameter added to bubble test configuration.The addition of
fluid_pp(2)%D_vwith value 0.242e-4 properly extends the test configuration for Eulerian bubble models, consistent with the migration of vapor diffusivity from lag_params to per-fluid parameters.
850-850: LGTM! D_v parameter added to Lagrangian bubble test configuration.The addition of
fluid_pp(2)%D_vwith value 2.5e-5 properly extends the test configuration for Lagrangian bubble models.src/simulation/m_global_parameters.fpp (1)
685-685: LGTM! D_v initialized with default value.The initialization of
fluid_pp(i)%D_v = dflt_realfollows the established pattern for other fluid properties and ensures proper default values before user configuration is applied.examples/1D_qbmm/case.py (1)
28-28: LGTM! D_v parameter added to example configuration.The addition of vapor diffusivity D_v for the bubble gas (fluid 2) properly extends the example configuration. The value 0.242e-4 is consistent with typical air vapor diffusivity values.
Also applies to: 167-167
toolchain/mfc/run/case_dicts.py (4)
154-154: LGTM! D_v added to PRE_PROCESS fluid parameters.The addition of "D_v" to the real attributes list for
fluid_ppin PRE_PROCESS properly extends the parameter schema to support vapor diffusivity as a per-fluid property.
352-354: LGTM! Obsolete diffcoefvap parameter removed.The removal of 'diffcoefvap' from
lag_paramsreal variables is correct, as this parameter has been migrated to the per-fluidfluid_pp(...)%D_vstructure.
417-417: LGTM! D_v added to SIMULATION fluid parameters.Consistent with PRE_PROCESS changes, properly extending the SIMULATION parameter schema.
537-537: LGTM! D_v added to POST_PROCESS fluid parameters.Completes the consistent addition of D_v across all three processing stages.
src/simulation/m_bubbles_EL.fpp (1)
181-181: LGTM! D_v properly normalized in lagrange inputs.The nondimensionalization of
fluid_pp(id_bubbles)%D_vby(x0*c0)correctly applies the reference scaling for vapor diffusivity in the Lagrangian bubble model initialization.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 15 files
Prompt for AI agents (all 3 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="examples/3D_lagrange_shbubcollapse/case.py">
<violation number="1" location="examples/3D_lagrange_shbubcollapse/case.py:1">
The shebang now ends with a carriage return, so executing the script directly will fail because `/usr/bin/env` looks for an interpreter named `python3\r`. Remove the `\r` so the shebang is POSIX-compatible.</violation>
</file>
<file name="examples/3D_lagrange_bubblescreen/case.py">
<violation number="1" location="examples/3D_lagrange_bubblescreen/case.py:1">
The shebang now has a Windows-style carriage return, so executing `./case.py` on Unix fails because `/usr/bin/env` cannot find `python3\r`. Use a Unix newline for the interpreter line.</violation>
</file>
<file name="src/simulation/m_bubbles_EL.fpp">
<violation number="1" location="src/simulation/m_bubbles_EL.fpp:181">
Do not overwrite the global fluid diffusivity with the nondimensional value here; other modules (e.g., m_helper) still expect fluid_pp%D_v to stay in dimensional units, so this line now feeds a scaled diffusivity into the common Peclet-number calculations.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
There was a problem hiding this comment.
2 issues found across 15 files
Prompt for AI agents (all 2 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/common/m_helper.fpp">
<violation number="1" location="src/common/m_helper.fpp:161">
`R_uni` is referenced without being imported into `m_helper`, so the code no longer compiles after removing the local `Ru` constant.</violation>
</file>
<file name="src/simulation/m_bubbles_EL.fpp">
<violation number="1" location="src/simulation/m_bubbles_EL.fpp:181">
Existing cases still provide the diffusion coefficient via `lag_params%diffcoefvap`, so `fluid_pp(num_fluids)%D_v` stays at its default sentinel and `PeG` now divides by an unset value. Add a fallback that populates `fluid_pp% D_v` from `lag_params` when it has not been provided to avoid dividing by zero for current inputs.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| ! gas constants | ||
| R_n = Ru/M_n | ||
| R_v = Ru/M_v | ||
| R_n = R_uni/M_n |
There was a problem hiding this comment.
R_uni is referenced without being imported into m_helper, so the code no longer compiles after removing the local Ru constant.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/common/m_helper.fpp, line 161:
<comment>`R_uni` is referenced without being imported into `m_helper`, so the code no longer compiles after removing the local `Ru` constant.</comment>
<file context>
@@ -154,15 +151,15 @@ contains
! gas constants
- R_n = Ru/M_n
- R_v = Ru/M_v
+ R_n = R_uni/M_n
+ R_v = R_uni/M_v
! phi_vn & phi_nv (phi_nn = phi_vv = 1)
</file context>
| R_v = (R_uni/fluid_pp(id_bubbles)%M_v)*(T0/(c0*c0)) | ||
| R_n = (R_uni/fluid_pp(id_host)%M_v)*(T0/(c0*c0)) | ||
| lag_params%diffcoefvap = lag_params%diffcoefvap/(x0*c0) | ||
| fluid_pp(id_bubbles)%D_v = fluid_pp(id_bubbles)%D_v/(x0*c0) |
There was a problem hiding this comment.
Existing cases still provide the diffusion coefficient via lag_params%diffcoefvap, so fluid_pp(num_fluids)%D_v stays at its default sentinel and PeG now divides by an unset value. Add a fallback that populates fluid_pp% D_v from lag_params when it has not been provided to avoid dividing by zero for current inputs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/simulation/m_bubbles_EL.fpp, line 181:
<comment>Existing cases still provide the diffusion coefficient via `lag_params%diffcoefvap`, so `fluid_pp(num_fluids)%D_v` stays at its default sentinel and `PeG` now divides by an unset value. Add a fallback that populates `fluid_pp% D_v` from `lag_params` when it has not been provided to avoid dividing by zero for current inputs.</comment>
<file context>
@@ -178,7 +178,7 @@ contains
R_v = (R_uni/fluid_pp(id_bubbles)%M_v)*(T0/(c0*c0))
R_n = (R_uni/fluid_pp(id_host)%M_v)*(T0/(c0*c0))
- lag_params%diffcoefvap = lag_params%diffcoefvap/(x0*c0)
+ fluid_pp(id_bubbles)%D_v = fluid_pp(id_bubbles)%D_v/(x0*c0)
ss = fluid_pp(id_host)%ss/(rho0*x0*c0*c0)
mul0 = fluid_pp(id_host)%mul0/(rho0*x0*c0)
</file context>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/documentation/case.md (1)
748-748: Add descriptive documentation for the new D_v parameter.The
D_vparameter is now added to the table (line 748), but the explanatory bullet-point block (lines 765–768) does not mention it. Users need clarity on when this parameter is used and how it relates to the bubble models.Consider updating the descriptive text to document
D_valongside the other vapor properties. For example:- `mu_l0`, `ss`, and `pv`, `gamma_v`, `M_v`, `mu_v`, `k_v`, and `cp_v` specify simulation parameters for the non-polytropic gas compression model. + `mu_l0`, `ss`, and `pv`, `gamma_v`, `M_v`, `mu_v`, `k_v`, `cp_v`, and `D_v` specify simulation parameters for the non-polytropic gas compression model. `mu_l0`, `ss`, and `pv` correspond to the liquid viscosity, surface tension, and vapor pressure, respectively. - `gamma_v`, `M_v`, `mu_v`, `k_v`, and `cp_v` specify the specific heat ratio, molecular weight, viscosity, thermal conductivity and specific heat capacity of a chosen component (`cp_v` only for ``bubbles_lagrange = 'T'``). + `gamma_v`, `M_v`, `mu_v`, `k_v`, `cp_v`, and `D_v` specify the specific heat ratio, molecular weight, viscosity, thermal conductivity, specific heat capacity (`cp_v` only for ``bubbles_lagrange = 'T'``), and vapor diffusivity of a chosen component.Also applies to: 765-768
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/documentation/case.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Code Cleanliness Check
- GitHub Check: Github (ubuntu, no-mpi, single, no-debug, false)
- GitHub Check: Github (ubuntu, mpi, no-debug, true)
- GitHub Check: Github (ubuntu, mpi, debug, true)
- GitHub Check: Github (ubuntu, mpi, debug, false)
- GitHub Check: Github (ubuntu, mpi, no-debug, false)
- GitHub Check: Coverage Test on CodeCov
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Build & Publish
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1062 +/- ##
=======================================
Coverage 44.35% 44.36%
=======================================
Files 71 71
Lines 20587 20588 +1
Branches 1993 1993
=======================================
+ Hits 9132 9134 +2
+ Misses 10310 10309 -1
Partials 1145 1145 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
User description
User description
Description
In the broader context, I'm working on integrating the initialization process and generalizing the normalization of EE/EL bubbles. This PR is the first step toward reducing redundancy and inconsistency across the bubble modules.
Rudefined ins_initialize_nonpoly, which is already defined as 'R_uniinm_constants.fpp`D_mdefined ins_initialize_nonpoly, which is an input parameterlag_params%diffcoefvapused for EL bubbles. Moved it tofluid_pp%D_vto make it available for both of EE/EL bubbles.Examples and tests are updated accordingly.
Type of change
Please delete options that are not relevant.
Scope
If you cannot check the above box, please split your PR into multiple PRs that each have a common goal.
How Has This Been Tested?
Test Configuration:
Checklist
docs/)examples/that demonstrate my new feature performing as expected.They run to completion and demonstrate "interesting physics"
./mfc.sh formatbefore committing my codeIf your code changes any code source files (anything in
src/simulation)To make sure the code is performing as expected on GPU devices, I have:
nvtxranges so that they can be identified in profiles./mfc.sh run XXXX --gpu -t simulation --nsys, and have attached the output file (.nsys-rep) and plain text results to this PR./mfc.sh run XXXX --gpu -t simulation --rsys --hip-trace, and have attached the output file and plain text results to this PR.PR Type
Bug fix, Enhancement
Description
Moved vapor diffusivity from
lag_params%diffcoefvaptofluid_pp%D_vReplaced hardcoded gas constant
RuwithR_unifrom constantsUpdated all related initialization, MPI broadcast, and example configurations
Diagram Walkthrough
File Walkthrough
5 files
Add vapor diffusivity field to physical parametersInitialize D_v field in fluid parametersAdd D_v to MPI broadcast for fluid parametersInitialize D_v field in fluid parametersAdd D_v to MPI broadcast for fluid parameters4 files
Replace hardcoded constants with centralized definitionsReplace lag_params diffusivity with fluid_pp D_vRemove diffcoefvap, add D_v to fluid parametersRemove diffcoefvap, add D_v to MPI broadcast4 files
Update example to use fluid_pp D_v parameterUpdate example to use fluid_pp D_v parameterUpdate example to use fluid_pp D_v parameterRemove diffcoefvap, add D_v to parameter definitions2 files
Remove diffcoefvap from test case defaultsUpdate test cases to use fluid_pp D_v parameterCodeAnt-AI Description
Treat vapor diffusivity as a fluid property for bubble calculations
What Changed
Impact
✅ Consistent vapor diffusion across bubble setups✅ Accurate mass-transfer behavior in parallel bubble simulations✅ Example cases capture vapor diffusivity input requirements💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Refactor
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.